Skip to content

r/aws_bedrockagentcore_harness: add LiteLLM model and full skill union#48656

Open
jesseturner21 wants to merge 3 commits into
hashicorp:mainfrom
jesseturner21:agentcore/harness-litellm-skill
Open

r/aws_bedrockagentcore_harness: add LiteLLM model and full skill union#48656
jesseturner21 wants to merge 3 commits into
hashicorp:mainfrom
jesseturner21:agentcore/harness-litellm-skill

Conversation

@jesseturner21

@jesseturner21 jesseturner21 commented Jun 26, 2026

Copy link
Copy Markdown

Description

Adds two harness model/skill features that exist in the Bedrock AgentCore control plane API but were missing from the Terraform schema.

  • model.litellm_model_config — new model union variant (model_id, api_base, api_key_arn, max_tokens, temperature, top_p).
  • skill — rebuilt from a path-only block into the full SDK union: path, s3, git (with auth), and aws_skills.

⚠️ Depends on #48655 — review that first

This PR is built on top of #48655 (the memory Unsupported Type fix). GitHub cross-fork PRs cannot target a fork branch as their base, so this PR is opened against main and its diff currently also contains the memory-fix commit from #48655. Once #48655 merges, that commit becomes part of main and this PR's diff will collapse to only the LiteLLM + skill changes.

Reviewers: the only commit unique to this PR is r/aws_bedrockagentcore_harness: add LiteLLM model and full skill union. The memory commit belongs to #48655.

Why the dependency is real: the service provisions a managed memory configuration by default, so a harness can't be created — and these acceptance tests can't pass — without #48655's flatten fix.

Split from #48556

Part of breaking up #48556 (size/XL) into focused PRs.

Honest caveats

Testing

TF_ACC=1 go test -v -timeout 60m ./internal/service/bedrockagentcore/ \
  -run "TestAccBedrockAgentCoreHarness_(model_litellm|skill_git)$"

--- PASS: TestAccBedrockAgentCoreHarness_skill_git (353.64s)
--- PASS: TestAccBedrockAgentCoreHarness_model_litellm (364.11s)
PASS
ok  github.com/hashicorp/terraform-provider-aws/internal/service/bedrockagentcore  364.271s

go build, go vet, gofmt clean.

Relations

Part of splitting #48556. Closes #48540. Depends on #48655.

@github-actions

Copy link
Copy Markdown
Contributor

Community Guidelines

This comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀

Voting for Prioritization

  • Please vote on this Pull Request by adding a 👍 reaction to the original post to help the community and maintainers prioritize it.
  • Please see our prioritization guide for additional information on how the maintainers handle prioritization.
  • Please do not leave +1 or other comments that do not add relevant new information or questions; they generate extra noise for others following the Pull Request and do not help prioritize the request.

Pull Request Authors

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions Bot added needs-triage Waiting for first response or review from a maintainer. documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/bedrockagentcore Issues and PRs that pertain to the bedrockagentcore service. size/XL Managed by automation to categorize the size of a PR. labels Jun 26, 2026
@jesseturner21 jesseturner21 force-pushed the agentcore/harness-litellm-skill branch from 2cffb83 to 9445e29 Compare June 26, 2026 16:16
@jesseturner21 jesseturner21 marked this pull request as ready for review June 26, 2026 16:21
@jesseturner21 jesseturner21 requested a review from a team as a code owner June 26, 2026 16:21
@dosubot dosubot Bot added the enhancement Requests to existing resources that expand the functionality or scope. label Jun 26, 2026
@justinretzolk justinretzolk removed the needs-triage Waiting for first response or review from a maintainer. label Jun 29, 2026
jesseturner21 and others added 3 commits July 10, 2026 19:23
- model.litellm_model_config: new model union variant (api_base, api_key_arn,
  max_tokens, model_id, temperature, top_p)
- skill: rebuilt from path-only into the full SDK union — path, s3, git
  (with auth), and aws_skills. skill.path widens from Required to Optional
  (backward-compatible).

Acceptance tests for the LiteLLM model and git skill source included.

Split out of hashicorp#48556.
… and float precision bugs

- listvalidator.ExactlyOneOf across the four *_model_config blocks
  (attached once; the framework tallies every sibling even when the
  attached block is null) catches multi-member configs (which previously
  silently dropped members and produced a perpetual diff) and empty
  model {} (which previously surfaced an internal
  "always an error in the provider" message) at plan time.
- stringvalidator.RegexMatches on harness_name enforces the service
  pattern ^[a-zA-Z][a-zA-Z0-9_]{0,39}$ offline; names starting with a
  digit or containing dashes previously validated cleanly and only
  failed at CreateHarness.
- max_tokens is Optional+Computed with UseStateForUnknown. The Update
  API retains the existing value on omission ("if not specified, the
  existing value is retained"), so state absorbs the retained server
  value instead of showing a perpetual "2000 -> null" diff after
  removing max_tokens from configuration.
- temperature and top_p on all four model configs are modeled as
  Float64 (schema + model struct). AutoFlex's flattenFloat32 routes
  float32 -> types.Float64 through decimal.NewFromFloat32 (comment:
  "Avoid loss of equivalence"), so imports round-trip cleanly
  (temperature = 0.7 stays 0.7, not 0.699999988079071) and no
  ImportStateVerifyIgnore workaround is needed.

Add acctests: harness_name regex ExpectError, model-union ExactlyOneOf
ExpectError (multi-member and empty), and a max_tokens set->unset
transition asserting PostApplyPostRefresh ExpectEmptyPlan (no
perpetual diff).
Dropping the duplicated memory commit (hashicorp#48655's 7151e9d) resurfaces the base
retrieval_config.relevance_score (Float32), so the float32validator import is
still needed. The model-config float-precision (Float64) work stays intact.
@aidandaly24 aidandaly24 force-pushed the agentcore/harness-litellm-skill branch from 918226a to 0f7465b Compare July 10, 2026 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/bedrockagentcore Issues and PRs that pertain to the bedrockagentcore service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aws_bedrockagentcore_harness: skill block should support s3, git, and container skill types (currently only container path is supported)

3 participants